fix(reload): stale syntax highlights after r key / watch reload#146
fix(reload): stale syntax highlights after r key / watch reload#146eduwass wants to merge 2 commits intomodem-dev:mainfrom
Conversation
The highlight cache keyed by appearance + file.id served stale HAST nodes after reload because the file ID is path-based and doesn't change when content changes. Switch to a content-addressed cache key using a patch fingerprint, add promise race guards, and cap the cache at 150 entries.
Greptile SummaryThis PR fixes a stale syntax highlight bug after Key changes:
Confidence Score: 4/5Safe to merge for git-backed diffs; introduces a reload regression for The core cache fix in src/ui/App.tsx — the Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User (r key)
participant App as App.tsx
participant AH as AppHost.tsx
participant HL as useHighlightedDiff.ts
participant Cache as SHARED_CACHE
U->>App: press "r"
App->>App: refreshCurrentInput()
App->>AH: onReloadSession(nextInput, { sourcePath: sourceLabel })
AH->>AH: loadAppBootstrap(input, { cwd: sourcePath })
AH->>App: setActiveBootstrap(nextBootstrap)
Note over App,HL: Component re-renders with new bootstrap
App->>HL: useHighlightedDiff({ file: newFile, ... })
HL->>HL: buildCacheKey(appearance, file)
HL->>Cache: has(newCacheKey)?
Cache-->>HL: false (content changed, new key)
HL->>HL: loadHighlightedDiff(file) to promise
HL->>Cache: PROMISES.set(newCacheKey, promise)
HL->>HL: effectPromise.then(result)
HL->>HL: commitHighlightResult(key, promise, result)
HL->>HL: PROMISES.get(key) === promise? true
HL->>Cache: CACHE.set(newCacheKey, result)
HL->>HL: setHighlighted(result)
Note over HL,Cache: Old promise resolves late
HL->>HL: commitHighlightResult(oldKey, oldPromise, stale)
HL->>HL: PROMISES.get(oldKey) !== oldPromise, false
Note over HL: Stale result discarded
Reviews (1): Last reviewed commit: "fix(reload): stale syntax highlights aft..." | Re-trigger Greptile |
- Guard sourcePath on git-backed input kinds only (diff/difftool use display strings like "file compare" as sourceLabel, not paths) - Strengthen patchFingerprint to sample start, middle, and end of the patch to avoid collisions on edits deep in large files - Remove redundant effectPromise && guards inside .then()/.catch()
|
Addressing review feedback (67a3223): P1 — P2 — Weak fingerprint collision: Fixed. P2 — Redundant All 25 tests pass (23 existing + 2 new regression tests). Typecheck clean. |
Summary
Reload (
rkey and--watchmode) shows stale diff content for git working tree reviews. After editing a file and reloading, the old content persists in the UI even thoughgit diffreturns the correct updated output.Root cause
The syntax highlight cache in
useHighlightedDiff.tsuses${appearance}:${file.id}as its cache key. For git-backed diffs,file.idis derived from the file path (e.g.patch:0:readme.md), not the content. When a reload rebuilds the changeset, the file ID stays the same because the path hasn't changed — only the content has. The cached HAST nodes from the previous render are served, and sincebuildStackRows/buildSplitRowsread line content from the highlighted output (not the raw file metadata), the stale text appears in the UI.Reproduction
hunk diffin any git repo with uncommitted changesrto reload (or use--watch)Fix
Switch the highlight cache from identity-based keys to content-addressed keys:
buildCacheKey()includes apatchFingerprint()derived fromfile.patch, so changed files get a new key automatically while unchanged files keep their cache hit across reloads. No global cache flush needed.commitHighlightResult()only writes to the cache if the promise is still the active one registered for that key. Prevents a superseded or late-resolving async highlight from overwriting a newer entry.enforceCacheLimit()caps the result cache at 150 entries, preventing unbounded growth during long--watchsessions with many file edits.refreshCurrentInputnow passesbootstrap.changeset.sourceLabelassourcePathso git reload resolves the correct repo root (previously fell back toprocess.cwd()which could differ).Approaches considered and rejected
setAppVersionbump): Fixes staleness by clearing all component state, but destroys scroll position, selection, filter text, sidebar width, and all other UI state. Unacceptable UX tradeoff.highlightedstate viauseEffecton file reference change: The file object IS a new reference after reload, but theuseLayoutEffectbails out early athighlightedCacheKey === appearanceCacheKeybecause the key format was identity-based. Fixing the key is the right layer.Test plan
test/reload-bug.test.tsx— two tests covering file-pair diffs and git working tree diffsapp-interactionstests pass